✨ QDMI Environment Implementation #154

kayaercument · 2025-03-24T16:32:08Z

Description

This PR bundles the required implementations to retrieve environmental telemetry data using QDMI.

Checklist

The pull request only contains commits that are related to it.
Documentation and tests are created
The project's style is followed.

Closes: #152

Documentation will be created after details and naming are finalised.

burgholzer · 2025-03-24T21:26:02Z

Hey @kayaercument 👋🏼
Thanks for this initial proposal!
In order to evaluate its feasibility, could you please also add tests for the new feature that show it in action? (Very similar to what we already have for the existing functionality)
Code that compiles and "does something" is much easier to judge than a simple interface addition.
We will definitely need some bike shedding on the naming scheme, but I think that is inevitable anyway and should be a simple refactor in any case.
This looks like a solid start!

(I think I can fix the coverage uploads from forks not working. let me check)

mnfarooqi

@kayaercument
Here are a few minor comments while you are working on the tests.

include/qdmi/constants.h

include/qdmi/device.h

…c and their tests

kayaercument · 2025-04-09T15:18:14Z

Dear @echavarria-lrz and @burgholzer,

Please review the work. I will write the documentation after everything is finalised.

ystade

@kayaercument thanks for the proposal of integrating the query of environmental data into QDMI. I took the liberty and added few questions inline in the code. Those are open questions more on the fundamental side. Hence, I think it makes sense to find an asnwer to them first, before I or someone else goes through the code in detail.

Overall, all additions fit niceliy with the look and feel of QDMI. Just one minor point: Already for a review of the code, doxygen comments help a lot to understand what the functions are supposed to do. However, since you did not actively request a review and I just voluntarily took a look, I cannot really complain.

include/qdmi/client.h

examples/device/c/device.c

kayaercument · 2025-09-17T12:59:01Z

Hey @burgholzer and @ystade,

I and @mnfarooqi wanted to try use_oidc for the codecov report. However, it does not trigger. Could you please check?

burgholzer · 2025-09-17T13:05:08Z

Hey @burgholzer and @ystade,

I and @mnfarooqi wanted to try use_oidc for the codecov report. However, it does not trigger. Could you please check?

Yeah. Something seems to be broken with CI at the moment. I'll check.
Just fyi: we previously used OIDC for codecov already, but that didn't work either for forks and is unlikely to work because a fork does not have the right permissions to create a valid OIDC token for the main repository. Codecov actually tries to use the tokenless upload in these cases. However, it seems that something is messed up in some configuration which is why it still expects a token in these cases. Eventually, we might have to open a support ticket with them as it might very well be an issue that only they can fix.

burgholzer · 2025-09-17T13:26:27Z

CI is running again now. For some reason, the CI workflow was disabled 🤷🏼

mnfarooqi · 2025-09-17T14:52:43Z

Hey @burgholzer and @ystade,
I and @mnfarooqi wanted to try use_oidc for the codecov report. However, it does not trigger. Could you please check?

Yeah. Something seems to be broken with CI at the moment. I'll check. Just fyi: we previously used OIDC for codecov already, but that didn't work either for forks and is unlikely to work because a fork does not have the right permissions to create a valid OIDC token for the main repository. Codecov actually tries to use the tokenless upload in these cases. However, it seems that something is messed up in some configuration which is why it still expects a token in these cases. Eventually, we might have to open a support ticket with them as it might very well be an issue that only they can fix.

It may be something to do with the settings in Codecov? For example:
https://docs.codecov.com/docs/codecov-tokens#uploading-without-a-token

burgholzer · 2025-09-17T14:56:18Z

Hey @burgholzer and @ystade,
I and @mnfarooqi wanted to try use_oidc for the codecov report. However, it does not trigger. Could you please check?

Yeah. Something seems to be broken with CI at the moment. I'll check. Just fyi: we previously used OIDC for codecov already, but that didn't work either for forks and is unlikely to work because a fork does not have the right permissions to create a valid OIDC token for the main repository. Codecov actually tries to use the tokenless upload in these cases. However, it seems that something is messed up in some configuration which is why it still expects a token in these cases. Eventually, we might have to open a support ticket with them as it might very well be an issue that only they can fix.

It may be something to do with the settings in Codecov? For example: https://docs.codecov.com/docs/codecov-tokens#uploading-without-a-token

By now I tripple checked all settings across the GitHub repository, the GitHub organization as well as codecov. They perfectly match a known-good setup we use over at the MQT where codecov reports from forks work just fine without any special handling.
At this point I am 99% sure this will require a support ticket with codecov.

ystade

Hi @kayaercument
@burgholzer asked me wether I can do another round of review as he couldn't find the time to do so. Hence, I started and got until the middle of the file include/qdmi/client.h (from the top in the "Files Changed" overview). Here in this file I realized that you do not seem to be done with the PR as a lot of my last comments lack a reaction. Consequently, I stopped in the middle and will wait for your ping when you wnt through the feedback. Thanks for the effort.

ystade · 2025-09-26T15:50:11Z

.github/workflows/ci.yml

@burgholzer What is the latest state here regarding the CodeCov setup? Should these changes be reverted?

The current state is #154 (comment)

We should not need to explicitly set any secrets or tokens in workflows here. Any respective changes can be reverted here or should be moved to a separate PR that gets merged before this PR does.

It is reverted. Have any support tickets been created?

No. Would you mind creating one?

ystade · 2025-09-26T15:50:25Z

.github/workflows/reusable-cpp-ci.yml

Same holds for this file as commented above.

It is reverted.

ystade · 2025-09-26T16:04:20Z

examples/device/device.cpp

+       param != QDMI_DEVICE_TELEMETRYSENSOR_QUERY_PARAMETER_CUSTOM3 &&
+       param != QDMI_DEVICE_TELEMETRYSENSOR_QUERY_PARAMETER_CUSTOM4 &&
+       param != QDMI_DEVICE_TELEMETRYSENSOR_QUERY_PARAMETER_CUSTOM5) ||
+      value == nullptr || (size != sizeof(decltype(query->start_time)))) {


Suggested change

value == nullptr || (size != sizeof(decltype(query->start_time)))) {

value == nullptr) {

The check (size != sizeof(decltype(query->start_time))) must be placed somewhere differently because also other information than of the start_time's type can be returned, e.g., a sensor pointer. This check should be handled in the respective cases.

examples/device/device.cpp

ystade · 2025-09-26T16:13:24Z

examples/driver/qdmi_example_driver.cpp

+  decltype(QDMI_device_session_query_telemetrysensor_property)
+      *device_session_query_telemetrysensor_property{};
+
+  decltype(QDMI_device_telemetrysensor_query_set_parameter)
+      *device_telemetrysensor_query_set_parameter{};
+
+  decltype(QDMI_device_session_create_device_telemetrysensor_query)
+      *device_session_create_device_telemetrysensor_query{};
+
+  decltype(QDMI_device_telemetrysensor_query_submit)
+      *device_telemetrysensor_query_submit{};
+
+  decltype(QDMI_device_telemetrysensor_query_get_results)
+      *device_telemetrysensor_query_get_results{};
+
+  decltype(QDMI_device_telemetrysensor_query_check_status)
+      *device_telemetrysensor_query_check_status{};
+
+  decltype(QDMI_device_telemetrysensor_query_wait)
+      *device_telemetrysensor_query_wait{};
+
+  decltype(QDMI_device_telemetrysensor_query_cancel)
+      *device_telemetrysensor_query_cancel{};
+
+  decltype(QDMI_device_telemetrysensor_query_free)
+      *device_telemetrysensor_query_free{};
+


Would you mind adding docstrings similar to the lines above?

It is added.

ystade · 2025-09-26T16:22:20Z

include/qdmi/client.h

+ * Additionally, the size of the buffer needed to retrieve the property is
+ * returned in @p size_ret if @p size_ret is not @c NULL.
+ *
+ * @note For example, to query the unit of an telemetry sensor, the following


Suggested change

* @note For example, to query the unit of an telemetry sensor, the following

* @note For example, to query the unit of a telemetry sensor, the following

As mentioned above, please also check the rest.

ystade · 2025-09-26T16:22:41Z

include/qdmi/client.h

+ * returned in @p size_ret if @p size_ret is not @c NULL.
+ *
+ * @note For example, to query the unit of an telemetry sensor, the following
+ code pattern


Again a weird line break.

ystade · 2025-09-26T16:24:43Z

include/qdmi/client.h

+ * size_t telemetrysensor_unit_size = 0;
+ * auto ret = QDMI_device_query_telemetrysensor_property(
+ *     device, telemetry_sensor, QDMI_TELEMETRYSENSOR_PROPERTY_UNIT, 0,
+ nullptr,


Please check for all other weird line breaks and fix them.

ystade · 2025-09-26T16:25:09Z

include/qdmi/client.h

+/** @defgroup client_telemetrysensor_query_interface \
+ *  QDMI Client Telemetry Sensor Query Interface
+ *  @brief Provides functions to query telemetry sensors.
+ *  @details An telemetry sensor query is a task submitted by a client to a


Suggested change

* @details An telemetry sensor query is a task submitted by a client to a

* @details A telemetry sensor query is a task submitted by a client to a

ystade · 2025-09-26T16:26:48Z

include/qdmi/client.h

+ * @see QDMI_Device_TelemetrySensor_Query for the device-side the telemetry
+ * sensor query handle.


Coud it be that there is one the too many?

Co-authored-by: Yannick Stade <[email protected]> Signed-off-by: Ercüment Kaya <[email protected]>

codecov · 2025-11-12T10:40:37Z

Codecov Report

❌ Patch coverage is 75.00000% with 52 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
examples/device/device.cpp	67.5%	38 Missing ⚠️
examples/driver/qdmi_example_driver.cpp	73.0%	14 Missing ⚠️

❌ Your patch check has failed because the patch coverage (75.0%) is below the target coverage (90.0%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (85.9%) is below the adjusted base coverage (88.6%). You can increase the head coverage or adjust the Removed Code Behavior.

@@            Coverage Diff            @@
##           develop    #154     +/-   ##
=========================================
- Coverage     89.1%   85.9%   -3.3%     
=========================================
  Files            6       6             
  Lines          712     920    +208     
  Branches       140     170     +30     
=========================================
+ Hits           635     791    +156     
- Misses          77     129     +52

Flag	Coverage Δ
cpp	`85.9% <75.0%> (-3.3%)`	⬇️

Files with missing lines	Coverage Δ
examples/fomac/example_fomac.cpp	`81.3% <100.0%> (+5.4%)`	⬆️
examples/fomac/example_fomac.hpp	`100.0% <ø> (ø)`
examples/driver/qdmi_example_driver.cpp	`85.9% <73.0%> (-3.6%)`	⬇️
examples/device/device.cpp	`87.3% <67.5%> (-6.7%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

burgholzer · 2025-11-19T17:06:04Z

I am moving this to the 1.3.0 milestone for now as there still seems to be ongoing discussions that need to be resolved and the changes on main have been pretty stable for a while.

kayaercument and others added 2 commits March 24, 2025 15:57

✨ Initial QDMI Environment impl

0887df6

🎨 pre-commit fixes

302c416

burgholzer assigned kayaercument Mar 24, 2025

burgholzer added minor Changes that warrant a minor version release feature New feature or feature request labels Mar 24, 2025

mnfarooqi reviewed Mar 25, 2025

View reviewed changes

include/qdmi/constants.h Outdated Show resolved Hide resolved

include/qdmi/constants.h Outdated Show resolved Hide resolved

include/qdmi/device.h Show resolved Hide resolved

kayaercument and others added 16 commits April 4, 2025 16:09

🎨 renaming definitions for qdmi environment

3aab49e

🎨 pre-commit fixes

52bd8d7

🚚 rename environment to environmentvariable

49cc0bf

✨ Implemention of QDMI Enviroment functions in example C device

3ba8a92

🎨 pre-commit fixes

6f658f6

✏️ fix typo

25abdd4

✨ additional functions and their implementations in example c device

98971ba

🎨 pre-commit fixes

98a8659

✨ additional definations in the client interface

c2b2935

🎨 pre-commit fixes

f88b60c

✨ Implemention of QDMI Enviroment functions in example CXX device

c1591fe

🎨 pre-commit fixes

4e7b8f8

✨ implementation of qdmi environment function in example driver, foma…

bd849ae

…c and their tests

🎨 pre-commit fixes

3858563

🎨 minor changes

3929d0a

🎨 pre-commit fixes

16afe77

mnfarooqi requested a review from burgholzer April 10, 2025 11:19

ystade requested changes Apr 25, 2025

View reviewed changes

include/qdmi/client.h Outdated Show resolved Hide resolved

include/qdmi/client.h Outdated Show resolved Hide resolved

HossamAhmed2 reviewed Apr 26, 2025

View reviewed changes

examples/device/c/device.c Outdated Show resolved Hide resolved

burgholzer added this to the v1.2.0 milestone May 2, 2025

burgholzer and others added 2 commits May 3, 2025 00:33

Merge branch 'develop' into 152-env-prop

9bbc775

✏️ initial documentation for qdmi_environment

7b3e8df

Merge branch 'develop' into 152-env-prop

d2f3321

burgholzer mentioned this pull request Sep 17, 2025

Fix device template test #225

Merged

8 tasks

Merge branch 'develop' into 152-env-prop

8c747b3

ystade added 2 commits September 26, 2025 18:10

🎨 Clean-up example implementation

3a3576d

🎨 Minor improvements in example driver

35dc853

ystade reviewed Sep 26, 2025

View reviewed changes

kayaercument and others added 12 commits September 30, 2025 10:57

⏪ revert codecov fix attempt

c7ffdc3

✏️ fix typos in the documents

e3467e6

📝 additional documentations

b4c416d

🚨 fix lint warning

d61e4e7

✏️ fix typos

abe1b30

✏️ corrects weird line breaks

891abf3

🎨 improve consistent usage

abbbbd5

Apply suggestions from code review

c7432aa

Co-authored-by: Yannick Stade <[email protected]> Signed-off-by: Ercüment Kaya <[email protected]>

🎨 pre-commit fixes

ecc8fcd

🎨 use ctime instead of chrono

b88ff2f

🎨 minor change

72cf9eb

Merge branch 'develop' into 152-env-prop

ddd73a1

ystade mentioned this pull request Oct 29, 2025

🧪 Testing code cov - not meant to be merge #232

Closed

8 tasks

Merge branch 'develop' into 152-env-prop

80628dd

burgholzer modified the milestones: v1.2.0, v1.3.0 Nov 19, 2025

burgholzer removed their request for review December 13, 2025 14:42

	value == nullptr \|\| (size != sizeof(decltype(query->start_time)))) {
	value == nullptr) {

	* @note For example, to query the unit of an telemetry sensor, the following
	* @note For example, to query the unit of a telemetry sensor, the following

	* @details An telemetry sensor query is a task submitted by a client to a
	* @details A telemetry sensor query is a task submitted by a client to a

		* @see QDMI_Device_TelemetrySensor_Query for the device-side the telemetry
		* sensor query handle.

✨ QDMI Environment Implementation #154

Are you sure you want to change the base?

✨ QDMI Environment Implementation #154

Uh oh!

Conversation

kayaercument commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

burgholzer commented Mar 24, 2025

Uh oh!

mnfarooqi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kayaercument commented Apr 9, 2025

Uh oh!

ystade left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kayaercument commented Sep 17, 2025

Uh oh!

burgholzer commented Sep 17, 2025

Uh oh!

burgholzer commented Sep 17, 2025

Uh oh!

mnfarooqi commented Sep 17, 2025

Uh oh!

burgholzer commented Sep 17, 2025

Uh oh!

ystade left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 12, 2025

Codecov Report

kayaercument commented Mar 24, 2025 •

edited

Loading